Skip to content

updated requires to reflect functional changes in package #6

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
merged 5 commits into from
Feb 12, 2023
Merged

updated requires to reflect functional changes in package #6

merged 5 commits into from
Feb 12, 2023

Conversation

@antonhibl
Copy link
Contributor

@antonhibl antonhibl commented Feb 12, 2023 

it looks like the openai-completion--data-choices and
openai-completion--get-choices functions were renamed to
openai--data-choices and openai--get-choices in the openai package. I also
changed the require to openai vs openai-collections as that is the full
package name. addresses #5.

@antonhibl
updated requires to reflect functional changes in package
000f38e 
    it looks like the openai-completion--data-choices and
    openai-completion--get-choices functions were renamed to
    openai--data-choices and openai--get-choices in the openai package. I also
    changed the require to openai vs openai-collections as that is the full
    package name. addresses emacs-openai#5.
@antonhibl
Copy link
Contributor Author

antonhibl commented Feb 12, 2023

addresses #5

@antonhibl antonhibl mentioned this pull request Feb 12, 2023
Closed
jcs090218
jcs090218 reviewed Feb 12, 2023
View reviewed changes
Copy link
Member

@jcs090218 jcs090218 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks. I forgot to change the API here. 😅

@antonhibl
Copy link
Contributor Author

antonhibl commented Feb 12, 2023 •
edited
Loading

@jcs090218 it looks like the (require 'openai-completions) line is throwing off the CI system, are we sure we need that line f we have (require 'openai) above it? Importing openai should handle importing the same things I believe although correct me if I am wrong because you are the author.

@antonhibl
Copy link
Contributor Author

antonhibl commented Feb 12, 2023

let me know if you see the same CI issues, I just removed the (require 'openai-completions) line from this commit since it cause the CI to fail.

@jcs090218
Copy link
Member

jcs090218 commented Feb 12, 2023

I think we only need openai-completion since it already require openai by itself. See https://github.com/emacs-openai/openai/blob/e1a7fa234faa1130e997a942e96fe18d9785d0f3/openai-completion.el#L29

@antonhibl
updated requires, typo error
ee3e4f8 
    I had missed the s on the end of completions before 🤦 in `(require
    'openai-completions)`.
@antonhibl
typo
824aa82 
    completions -> completion
@antonhibl
Copy link
Contributor Author

antonhibl commented Feb 12, 2023

Makes sense, it should be fixed now. Not sure why the CI failed before since in this case it is a valid require for openai-completion @jcs090218

@jcs090218
Copy link
Member

jcs090218 commented Feb 12, 2023

I think you added the extra s behind. https://github.com/emacs-openai/codegpt/actions/runs/4158060485/jobs/7193041221#step:5:125

@antonhibl
Copy link
Contributor Author

antonhibl commented Feb 12, 2023

that makes sense, oops 😅

@jcs090218
Copy link
Member

jcs090218 commented Feb 12, 2023

No worries! Thanks for your contribution!

Feel free to make more contributions! Looking forward to collaborating with others! ;)

@jcs090218 jcs090218 merged commit 55e5b54 into emacs-openai:master Feb 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jcs090218 jcs090218 jcs090218 approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@antonhibl @jcs090218